Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix mtv calculation for both polygon-polygon as well as polygon-circle intersections. #24

Merged
merged 5 commits into from
Oct 29, 2024

Conversation

d3rped
Copy link
Contributor

@d3rped d3rped commented Sep 1, 2024

This fixes the potential incorrect direction of the MTV.

While testing I noticed that polygon-circle intersections have a similar issue once the circles center is inside the polygon.
A naive approach would be to do the same check and set the the magnitude to 2*r - smallest.Magnitude and invert it.

if cp.Center().Sub(other.position).Dot(smallest.Unit()) < 0 {
    smallest = smallest.Unit().Scale(-other.radius * 2 + smallest.Magnitude())
}

But that only works if the intersecting points are on the same edge.

@d3rped d3rped changed the title FIX: MTV for polygons now has the right direction. [DRAFT] fix: MTV for polygons now has the right direction. Sep 2, 2024
@d3rped
Copy link
Contributor Author

d3rped commented Sep 2, 2024

I noticed that polygon-polygon collisions have a similar issues to the one circles have.
Center point outside of lager polygon:
centerOut
Center point inside of larger polygon:
centerIn

I want to properly solve all the issues before this can be merged.

@d3rped
Copy link
Contributor Author

d3rped commented Sep 3, 2024

I've fixed the polygon-polygon mtv calculation.
Your implementation of the overlap function was faulty.
Checking for the min/max of both may be quicker but it does not handle intersections in which the min and max of one projection are contained by the other projection.
In the example I gave in my last comment the mtv becomes the width of the smaller polygon.

Regarding the polygon-circle mtv calculation.
The approach to only check against the polygons vertices and the center of the intersecting points is clever but can lead to wrong results.
I propose two different solutions:

  1. Only add the intersection center point to the checked vertices if it is not contained by the polygon.
    This means that there are some cases in which the mtv is not actually the minimum, but only in cases in which the current solution would give incorrect results anyways.

  2. Solve it the traditional way, that is:
    Project the circle against all polygon edges and find the minimum overlap.
    Keep the check against the polygons vertices (without the intersection center point)

@d3rped
Copy link
Contributor Author

d3rped commented Sep 3, 2024

For now I've implemented solution 2 as it does not have any corner cases.
Hope I didn't make any mistakes.

This should be ready for review now.

@d3rped d3rped changed the title [DRAFT] fix: MTV for polygons now has the right direction. fix: Fix mtv calculation for both polygon-polygon as well as polygon-circle intersections. Sep 3, 2024
@SolarLune
Copy link
Owner

Finally, haha! Excellent - thank you very much for the contribution! Really appreciate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants